-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
private string ExpandTildeSlash(string path) | ||
{ | ||
string tildeSlash = "~/"; | ||
if (path.StartsWith(tildeSlash) && !string.IsNullOrEmpty(_userHomeDirectory.Value)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -54,6 +56,19 @@ private IEnumerable<string> SearchPaths | |||
} | |||
} | |||
|
|||
private string ExpandTildeSlash(string path) | |||
{ | |||
string tildeSlash = "~/"; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
64bfb5b
to
c1def3d
Compare
@MattGertz for approval |
private string ExpandTildeSlash(string path) | ||
{ | ||
const string tildeSlash = "~/"; | ||
if (path.StartsWith(tildeSlash) && !string.IsNullOrEmpty(_userHomeDirectory.Value)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Why are we only handling "~/"? The bug only lists that one, but it can be the case that any of them impacts our search logic... This seems like something there should be a more general .NET API for resolving... |
c1def3d
to
8a56dfa
Compare
I agree @tannergooding . Path.Get full path should do the work. Or path.GetFullPathAsBash |
cc @JeremyKuhne Has this come up as a feature request on corefx before. Should we file something? |
I don't think anyone has asked to have a separate API. That is worth considering. Possibly we could add options? There are other things folks might want to tweak, such as 8.3 expansion. |
…ease/2.1.3xx-to-master * dotnet/release/2.1.3xx: Update to aspnetcore 2.1.0-preview1-28275 and react to feed layout changes (#8611) "ExternalRestoreSources" needs to be set in the docker container (#8602) Signing nupkg contents (Cli.Utils and MSBuildResolver) along with the rest of the compiled assemblies. Use satellites from roslyn package, not cli-deps-satellites Update to roslyn 2.7.0-beta3-62612-07 for 2.1.1xx Support TildeSlash expand (#8589) Port Kernel Version telemetry to preview1 Do not create a directory with a trailing space; it cannot be deleted by conventional methods. (#8587) Consume generic aspnetcore rpm installers Insert NuGet Build 4.6.0-rtm-4918 into cli Adding roslyn to automatic dependency flow through maestro. Fixing update dependency by using the new APIs. We broke this when we updated the version of VersionTools. MSBuild 15.6.81 Update SDK to 2.1.300-preview1-62608-07 MSBuild 15.6.80 Conflicts: build/DependencyVersions.props test/Microsoft.DotNet.ShellShim.Tests/ShellShimMakerTests.cs
Customer scenario
Consumer install a global tool on CLI that has the command name dotnet-mytool. The consumer cannot invoke the command by
dotnet mytool
.Bugs this fixes
https://github.com/dotnet/cli/issues/8450
Workarounds, if any
Add the full path of shim to PATH manually.
Risk
low
Performance impact
no
Root cause analysis
CLI command resolver does not implementation the same path logic as bash.
How was the bug found?
Issue filed on GitHub
fix https://github.com/dotnet/cli/issues/8450
Tilda expand is a bash behavior. I only add "~/" expand to $HOME
I cannot find a good way to test PATH. Considering we should not put things in CI's home directory. And if we mock a little bit, we are testing string class. So I only manually tested it.